Reviewed 1 of 1 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved

Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @pronebird)

Reviewed all commit messages.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @pronebird and @rablador)

ios/MullvadVPN/Notifications/Notification Providers/AccountExpiryInAppNotificationProvider.swift line 41 at r2 (raw file):

    var notificationDescriptor: InAppNotificationDescriptor? {
        guard

This is what I get when I log with an account that doesn't have time. It looks like the condition that was there previously was necessary.

Let's add tests to :

• Make sure we don't repeat that mistake again next time we modify this
• Make sure the behaviour is well defined

Reviewed 1 of 2 files at r2.
Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @rablador)

ios/MullvadVPN/Classes/CustomDateComponentsFormatting.swift line 32 at r2 (raw file):

        guard dateComponents.day != nil else {
            return NSLocalizedString(
                "ACCOUNT_EXPIRY_INAPP_NOTIFICATION_LESS_THAN_ONE_DAY",

Maybe the text identifier should better reflect where it comes from?

Reviewable status: 0 of 5 files reviewed, 2 unresolved discussions (waiting on @buggmagnet and @pronebird)

ios/MullvadVPN/Classes/CustomDateComponentsFormatting.swift line 32 at r2 (raw file):

Indeed

ios/MullvadVPN/Notifications/Notification Providers/AccountExpiryInAppNotificationProvider.swift line 41 at r2 (raw file):

Done.

Reviewable status: 0 of 5 files reviewed, 4 unresolved discussions (waiting on @buggmagnet, @pronebird, and @rablador)

ios/MullvadVPN/Classes/CustomDateComponentsFormatting.swift line 47 at r3 (raw file):

    }

    private static func isLessThanADayLeft(dateComponents: DateComponents) -> Bool {

nit: I would suggest of renaming that to isLessThanOneDay

Code snippet:

fileprivate extension DateComponents {
    var isLessThanOneDayLeft : Bool {
        let year = self.year ?? 0
        let day = self.day ?? 0

        return (year == 0) && (day == 0)
    }
}

ios/MullvadVPN/Notifications/Notification Providers/AccountExpiryInAppNotificationProvider.swift line 33 at r4 (raw file):

            let triggerDate,
            let duration = CustomDateComponentsFormatting.localizedString(
                from: Date(),

it's better to consider the now constant you've defined above

Reviewed 2 of 5 files at r3.
Reviewable status: 2 of 5 files reviewed, 3 unresolved discussions (waiting on @buggmagnet and @rablador)

Reviewed 2 of 5 files at r3, 1 of 1 files at r4.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @buggmagnet and @rablador)

Reviewed all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @mojganii and @rablador)

ios/MullvadVPN/Classes/CustomDateComponentsFormatting.swift line 47 at r3 (raw file):

The current name is fine

ios/MullvadVPN/Notifications/Notification Providers/AccountExpiryInAppNotificationProvider.swift line 13 at r4 (raw file):

import MullvadTypes

struct AccountExpiry {

Let's put this in a separate file

ios/MullvadVPN/Notifications/Notification Providers/AccountExpiryInAppNotificationProvider.swift line 19 at r4 (raw file):

        guard let expiryDate else { return nil }

        return Calendar.current.date(

It's better to use autoupdatingCurrent to track user made time changes.
...
Although considering we use current everywhere else, I will create a janitor task instead to change all references to autoupdatingCurrent

Reviewable status: 2 of 6 files reviewed, 4 unresolved discussions (waiting on @buggmagnet, @mojganii, and @pronebird)

ios/MullvadVPN/Classes/CustomDateComponentsFormatting.swift line 47 at r3 (raw file):

I'll change the name, but keep the function in CustomDateComponentsFormatting since it's probably only relevant in this context.

ios/MullvadVPN/Notifications/Notification Providers/AccountExpiryInAppNotificationProvider.swift line 13 at r4 (raw file):

Done.

ios/MullvadVPN/Notifications/Notification Providers/AccountExpiryInAppNotificationProvider.swift line 19 at r4 (raw file):

I'll keep it as-is then.

ios/MullvadVPN/Notifications/Notification Providers/AccountExpiryInAppNotificationProvider.swift line 33 at r4 (raw file):

Done.

Reviewed all commit messages.
Reviewable status: 2 of 6 files reviewed, 5 unresolved discussions (waiting on @mojganii, @pronebird, and @rablador)

ios/MullvadVPN.xcodeproj/project.pbxproj line 440 at r5 (raw file):

		7A6B4F592AB8412E00123853 /* TunnelMonitorTimings.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A6B4F582AB8412E00123853 /* TunnelMonitorTimings.swift */; };
		7A6F2FA52AFA3CB2006D0856 /* AccountExpiryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A6F2FA42AFA3CB2006D0856 /* AccountExpiryTests.swift */; };
		7A6F2FA72AFBB9AE006D0856 /* AccountExpiry.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A6F2FA62AFBB9AE006D0856 /* AccountExpiry.swift */; };

You forgot to make AccountExpiry part of the MullvadVPNTests target, the tests don't compile anymore

Reviewable status: 2 of 6 files reviewed, 2 unresolved discussions (waiting on @pronebird and @rablador)

Reviewed 1 of 5 files at r3, 3 of 4 files at r5.
Reviewable status: 5 of 6 files reviewed, 2 unresolved discussions (waiting on @mojganii, @pronebird, and @rablador)

ios/MullvadVPN.xcodeproj/project.pbxproj line 440 at r5 (raw file):

Went and fixed it after talking with @rablador since it was the only thing blocking this PR.